Skip to content

fix: address hot reloading issues #1102

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
fadi-george merged 3 commits into from
Jan 9, 2026
Merged

fix: address hot reloading issues #1102

fadi-george merged 3 commits into from
Jan 9, 2026

Conversation

@fadi-george
Copy link
Collaborator

@fadi-george fadi-george commented Dec 29, 2025 •
edited
Loading

One Line Summary

Closes #763

  • fixes minor formatting issues and improves the logic for handling hot restarts on Android and iOS

Details

  • use singelton pattenrs for most of the class instances (User, Notifications)
  • then in lifecycle inits we remove the old observers

Hot Restart Bug for iOS:

flutter-main-ios-bug.mov

Hot Restart Bug for Android:

flutter-main-android-bug.mov

Hot Restart Fix for iOS:

flutter-hotrestart-fix-ios.mov

Hot Restart Fix for Android:

flutter-hotrestart-fix-android.mov

Motivation

Manual testing

  • install flutter extension
  • you can use vscode / cursor (call action Flutter: Launch Emulator)
  • select ios or android device
  • go the example/lib/main.dart file then start debugging (you may need flutter extension)

Affected code checklist

  • Notifications
    • Display
    • Open
    • Push Processing
    • Confirm Deliveries
  • Outcomes
  • Sessions
  • In-App Messaging
  • REST API requests
  • Public API changes

Checklist

Overview

  • I have filled out all REQUIRED sections above
  • PR does one thing
    • If it is hard to explain how any codes changes are related to each other then it most likely needs to be more than one PR
  • Any Public API changes are explained in the PR details and conform to existing APIs

Testing

  • I have included test coverage for these changes, or explained why they are not needed
  • All automated tests pass, or I explained why that is not possible
  • I have personally tested this on my device, or explained why that is not possible

Final pass

  • Code is as readable as possible.
    • Simplify with less code, followed by splitting up code into well named functions and variables, followed by adding comments to the code.
  • I have reviewed this PR myself, ensuring it meets each checklist item
    • WIP (Work In Progress) is ok, but explain what is still in progress and what you would like feedback on. Start the PR title with "WIP" to indicate this.

This change is Reviewable

@fadi-george fadi-george requested a review from a team as a code owner December 29, 2025 21:52
@fadi-george fadi-george changed the title Fixes formatting and hot reload issues fix: address hot reloading issues Dec 29, 2025
@nan-li nan-li self-requested a review December 30, 2025 06:51
nan-li
nan-li requested changes Dec 30, 2025
View reviewed changes
Copy link
Contributor

@nan-li nan-li left a comment •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm I'm concerned about adding complexity for a dev-only scenario. Idk if there are any production side effects? Do you know if some lifecycle callbacks like onDetachedFromEngine could work instead?

@fadi-george
Copy link
Collaborator Author

fadi-george commented Jan 5, 2026

Hmm I'm concerned about adding complexity for a dev-only scenario. Idk if there are any production side effects? Do you know if some lifecycle callbacks like onDetachedFromEngine could work instead?

These just creates singletons and its a similar approach to what we did for react-native. hot-restarts (which is what the issue is about) doesnt trigger onDetachedFromEngine.

@fadi-george fadi-george requested review from jkasten2 and nan-li January 5, 2026 21:52
@nan-li
Copy link
Contributor

nan-li commented Jan 7, 2026 •
edited
Loading

The changes won't just apply to hot reload, but lifecycleInit() is called whenever clients call OneSignal.initialize("their_app_id"). Have we tested more flows not just for reload?

Did you test clicking from cold start?

nit: Add a comment explaining why this pattern is needed (for future maintainers who might question it)

nan-li
nan-li reviewed Jan 7, 2026
View reviewed changes
android/src/main/java/com/onesignal/flutter/OneSignalNotifications.java
}

private void registerClickListener() {
OneSignal.getNotifications().removeClickListener(this);
Copy link
Contributor

@nan-li nan-li Jan 7, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

not exactly related to this PR, but should the click listener also go in lifecycle init? may be related to the click cold start issues

Copy link
Collaborator Author

@fadi-george fadi-george Jan 9, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess the click listener is opt in so probably best not to mess with this behavior.

Copy link
Contributor

@nan-li nan-li Jan 9, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All listeners are opt in

@fadi-george fadi-george requested a review from nan-li January 9, 2026 00:59
@fadi-george fadi-george merged commit 8118f36 into main Jan 9, 2026
6 checks passed
@fadi-george fadi-george deleted the fg/hot-reload branch January 9, 2026 18:55
@fadi-george fadi-george mentioned this pull request Jan 9, 2026
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@nan-li nan-li nan-li approved these changes

@jkasten2 jkasten2 Awaiting requested review from jkasten2

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

[Bug]: OneSignal#onWillDisplayNotification getting called as many times as app gets hot restarted while debugging.

3 participants

@fadi-george @nan-li